security: prevent API key leakage in wrapper script logs (fixes #263)#313
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical security vulnerability where API keys were being printed to stdout in wrapper scripts, resulting in credentials being stored in plaintext log files (concoreout.txt). The fix removes direct API key printing and sanitizes debug URL output to prevent credential exposure while maintaining full functionality.
Changes:
- Removed
print(apikey)statements from all four wrapper scripts (demo/cwrap.py, demo/pwrap.py, ratc/cwrap.py, ratc/pwrap.py) - Replaced removed print statements with security-focused comments
- Sanitized debug URL prints in cwrap.py files to use
<APIKEY_HIDDEN>placeholder instead of actual API key
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| demo/pwrap.py | Removed API key print statement, replaced with security comment |
| demo/cwrap.py | Removed API key print statement and sanitized debug URL to hide API key |
| ratc/pwrap.py | Removed API key print statement, replaced with security comment |
| ratc/cwrap.py | Removed API key print statement and sanitized debug URL to hide API key |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Pls remove the comments "do not log for security reasons." Just removing the print(apikey) is sufficient. The inclusion was meant as a debug mechanism. |
GaneshPatil7517
force-pushed
the
fix/remove-api-key-logging
branch
from
37abab4 to
7803521
Compare
|
Thanks @pradeeban sir for the feedback! I've updated the PR removed the comments and just deleted the print(apikey) lines entirely. The branch has been force-pushed with the fix. |
|
@GaneshPatil7517 I am merging since this PR addresses a legitimate concern. But I think we should actually simply remove that log line altogether. It does not serve any purpose. Also, this code assumes the mediator is currently running in controlcore.org. It does not. |
Thanks for merging @pradeeban Sir Good point I can open a follow-up PR to remove that debug URL log line entirely since it doesn't serve a purpose. Happy to work on that if you'd like. |
2 participants
@pradeeban
Summary
This PR resolves Issue #263 by preventing API keys from being printed to stdout in wrapper scripts.
Previously, the API key was printed directly:
Since stdout is captured into
concoreout.txt, this resulted in credentials being stored in plaintext log files.Changes Made
Removed
print(apikey)from:demo/cwrap.pydemo/pwrap.pyratc/cwrap.pyratc/pwrap.pySanitized debug URL print in
demo/cwrap.pyandratc/cwrap.pyto use<APIKEY_HIDDEN>placeholder instead of the actual key.No logic changes API key is still read and used for HTTP requests, just never printed.
Security Impact
Scope
Testing
print(apikey)remains anywhere in the repository